Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sint] new presence subscription tests #481

Closed
wants to merge 4 commits into from

Conversation

guusdk
Copy link
Member

@guusdk guusdk commented Jun 30, 2021

This adds a few basic tests that verify that presence subscription stanzas are received 'as intended', notably when they include extension elements.

@guusdk
Copy link
Member Author

guusdk commented Jun 30, 2021

After grudgingly applying two other 'improvements', this PR fails once again on a Checkstyle-based rule. I'm giving up.

Copy link
Member

@Flowdalic Flowdalic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please find my review inline.

I think I've mentioned this before, but some of those tests are more towards testing server behavior and not really about integration tests. I guess a few simple ones are fine for Smack itself, but if you keep the integration tests coming that primarly test server behavior, then we really should think about establishing a separate xmpp-c2s-server-integration-test project where those tests reside in. Such a project can easily be based on, and exploit, Smack's integration test framework.

guusdk and others added 3 commits November 26, 2023 11:54
This adds a few basic tests that verify that presence subscription stanzas are received 'as intended', notably when they include extension elements.
@guusdk
Copy link
Member Author

guusdk commented Nov 26, 2023

I've applied the feedback and rebased to the current HEAD of the Master branch for good measure.

Using a low-level test is less straight-forward if it's desirable to be able to make use of provisioned user accounts, or a mix of connected and unconnected connections. I've experimented with using UnconnectedConnectionSource and taking usernames from the configuration, but could not get that to work. As a result, this test still depends on two connected resources, one of which is disconnected at the start of the tests. Those tests now do inherit from Low-Level, which is at least something, I suppose.

Copy link
Member

@Flowdalic Flowdalic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all of this belongs rather into smack-sint-server-extensions.

* @throws Exception on anything unexpected or undesired.
*/
@SmackIntegrationTest
public void testSubscriptionRequestOffline(final AbstractXMPPConnection conOne,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct me if I am wrong (for this and all potential following statements), but this seems to test primarily a server functionality (i.e., that a presence subscription test send while the recipient is offline, is delivered once the recipient comes online).

As such, it is probably not a good fit for Smack and instead for smack-sinttest-server-extensions

* @throws Exception on anything unexpected or undesired.
*/
@SmackIntegrationTest
public void testSubscriptionRequestOfflineWithExtension(final AbstractXMPPConnection conOne,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dito

@guusdk
Copy link
Member Author

guusdk commented Mar 8, 2024

The tests to be added in this PR are now being added to the smack-sint-server-extensions project instead: XMPP-Interop-Testing/smack-sint-server-extensions#6

@guusdk guusdk closed this Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants